Skip to content

Add complete service path to service module - #49

Merged
andersfugmann merged 1 commit into
andersfugmann:mainfrom
Nymphium:main
Oct 3, 2025
Merged

andersfugmann merged 1 commit into
andersfugmann:mainfrom
Nymphium:main

Conversation

@Nymphium

Copy link
Copy Markdown
Contributor

This PR adds a complete service name (package.path.Service) to a service module.

Previously, the full service name had to be constructed using an arbitrary method from that service. This was inconvenient, as the name is a property of the service itself, not a specific method.

let module Method = Some.Service.Method in
let service_path =
   match Method.package_name with
   | None -> Method.service_name
   | Some p -> p ^ "." Method.sevice_name

With this PR, the name can be accessed directly from the service module, simplifying the logic.

let service_path = Some.Service.package_service_name

@andersfugmann

andersfugmann commented Oct 1, 2025 •

Copy link
Copy Markdown
Owner

Thanks for the PR.

I was considering if this logic is not better placed in Ocaml_protoc_plugin.Service. E.g:

let service_name (module Method : Rpc) =
  match Method.package_name with
  | None -> Method.service_name
  | Some p -> Printf.sprintf "%s.%s" p Method.service_name

But that can only be used with an actual service, i.e:

let service_name = Ocaml_protoc_plugin.service_name (module Echo.Echo.Call)
...

What is the usecase for this? Each Method already have a val name : string that contains the grpc path, e.g. /echo.Echo/Call. Is is important to understand the service path explicitly?

@Nymphium

Nymphium commented Oct 1, 2025 •

Copy link
Copy Markdown
Contributor Author

To register a service, currently we have to construct its full name by pulling information from one of its specific methods. This creates an unnecessary dependency—we want to register the service as a whole, so it feels strange to rely on a single method like echo.Echo.Call just to figure out the service's name. It also means we end up writing the same boilerplate logic to build that name every time.

For example, this is the current approach with https://github.com/dialohq/ocaml-grpc:

let service = Grpc_eio.Server.Service.(v () |> call  |> handle_request)
let name =
    match Echo.Echo.Call.package_name with
    | None -> Echo.Echo.Call.service_name
    | Some path -> path ^ "." ^ Echo.Echo.Call.service_name
 in
Grpc_eio.Server.add_service ~name ~service

This PR would allow us to do this instead, which is much more straightforward:

Grpc_eio.Server.add_service
  ~name:Echo.Echo.package_service_name
  ~service

This new approach might be better because it decouples the service's identity from its methods and makes the service module the single source of truth for its own name. The code becomes more readable, and we get to remove that repetitive logic.

@andersfugmann

Copy link
Copy Markdown
Owner

Thanks for the explanation! Much appreciated and thanks for the PR.
I wonder if it would make sense to move the service_name and package_name into the service itself aswell to avoid the redundency (and mark the old definitions as deprecated to retain backward compatibility). Any thoughs on this?

@andersfugmann
andersfugmann merged commit 9f33972 into andersfugmann:main Oct 3, 2025
@Nymphium

Nymphium commented Oct 3, 2025

Copy link
Copy Markdown
Contributor Author

Thank you for approving it! I agree, that approach is much better 👍

andersfugmann added a commit to andersfugmann/opam-repository that referenced this pull request Sep 27, 2026
CHANGES:

- Add `package_service_name` to generated service modules, providing the
  complete protobuf service path directly (andersfugmann/ocaml-protoc-plugin#49, thanks @Nymphium)
- Use binary mode for the plugin's standard input and output on Windows
  (andersfugmann/ocaml-protoc-plugin#51, thanks @linsyking)
- Fix JSON decoder initialization when a message references an enum
  nested in another message (andersfugmann/ocaml-protoc-plugin#59, thanks @mbickers)
- Read the complete protoc plugin request instead of treating a short
  input read as end-of-file (andersfugmann/ocaml-protoc-plugin#60, thanks @jeong-sik)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants